implement sort#2446
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the sort2 method on the Array prototype! The structure is a solid start. However, this will require some changes as the core sorting logic is not working correctly and is missing a key feature from the native sort method.
Here are the two main issues to address:
- Incorrect Sorting Algorithm: The current implementation has logical flaws in its loops and comparison. The outer loop starts at
i = this.length, which causes an error becausethis[this.length]isundefined. A correct sorting algorithm, like bubble sort, typically compares adjacent elements (e.g.,this[j]andthis[j+1]) and swaps them if they're out of order. - Missing Default Behavior: The native
sortmethod works even without acompareFunctionby defaulting to string comparison. Your implementation will throw an error ifcompareFunctionisundefined. Please add a default comparator to handle this case.
Once these issues are resolved, your custom sort method will be fully functional. Keep up the great effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -6,7 +6,22 @@ | |||
| function applyCustomSort() { | |||
| [].__proto__.sort2 = function(compareFunction) { | |||
There was a problem hiding this comment.
The native Array.prototype.sort method has a default behavior for when no compareFunction is provided (it converts elements to strings and compares them). Your implementation will throw an error if called without an argument. Consider adding a default comparator if compareFunction is undefined.
| for (let i = this.length; i > 1; i--) { | ||
| for(let j = 0; j < i; j++) { | ||
| if (compareFunction(this[j], this[i]) < 0) { | ||
| swap(this, i, j); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This sorting algorithm has a couple of logical issues that prevent it from working correctly:
- The outer loop starts with
i = this.length, which will cause an error becausethis[this.length]isundefined. Array indices only go up tolength - 1. - The comparison logic is flawed. You are comparing each element
this[j]against a single elementthis[i]. A common sorting algorithm like bubble sort works by comparing adjacent elements (e.g.,this[j]andthis[j + 1]) and swapping them if they are in the wrong order.
2 participants
No description provided.